Skip to content

Remove autdated ASSERT fixing #13164 - #13165

Merged
Swiftb0y merged 2 commits into
mixxxdj:2.4from
daschuer:gh13164
Aug 16, 2024
Merged

Remove autdated ASSERT fixing #13164#13165
Swiftb0y merged 2 commits into
mixxxdj:2.4from
daschuer:gh13164

Conversation

@daschuer

@daschuer daschuer commented Apr 22, 2024

Copy link
Copy Markdown
Member

fixes #13164

@daschuer daschuer added this to the 2.4.1 milestone Apr 22, 2024
@daschuer daschuer modified the milestones: 2.4.1, 2.4.2 May 6, 2024
@daschuer

daschuer commented May 6, 2024

Copy link
Copy Markdown
Member Author

cleaning up the 2.4.1 milestone from this hard to reproduce issue

// FIXME: This should never happen but it does. But why??
return QVariant();
}
VERIFY_OR_DEBUG_ASSERT(siblingColumn != index.column()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be replaced with a if instead of removing the condition check entirely like above? Reading the comment, since it seems the early return was here to prevent infinite recursion. Or is that comment incorrect/out of date?

@daschuer

Copy link
Copy Markdown
Member Author

I have made rawValue() pure virtual to show that we have no infinitive recursive call issue.

@Swiftb0y Swiftb0y left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🤷

@daschuer

Copy link
Copy Markdown
Member Author

@Swiftb0y merge?

@Swiftb0y

Copy link
Copy Markdown
Member

I guess 🤷

@Swiftb0y
Swiftb0y merged commit a09448e into mixxxdj:2.4 Aug 16, 2024
@ronso0

ronso0 commented Aug 20, 2024

Copy link
Copy Markdown
Member

I could repro it finally and verify the fix! (with the location column I guess)
Merging to 2.5 now (#13584)

@daschuer
daschuer deleted the gh13164 branch April 24, 2025 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants